Skip to content

Clean up module outputFile calculation and move it to the specifications#6967

Open
alfonso-noriega wants to merge 1 commit intomainfrom
03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications
Open

Clean up module outputFile calculation and move it to the specifications#6967
alfonso-noriega wants to merge 1 commit intomainfrom
03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications

Conversation

@alfonso-noriega
Copy link
Copy Markdown
Contributor

@alfonso-noriega alfonso-noriega commented Mar 10, 2026 •
edited
Loading

WHY are these changes introduced?

The outputFileName property was previously calculated dynamically using a getter method that relied on the extension's build configuration mode. This approach was inflexible and didn't allow for extension-specific customization of output file names.

WHAT is this pull request doing?

  • Converts outputFileName from a computed getter to a direct property on ExtensionInstance
  • Adds an optional getOutputFileName method to the ExtensionSpecification interface
  • Implements getOutputFileName in all UI extension specifications to return ${extension.handle}.js
  • Implements getOutputFileName in function extensions to return index.wasm
  • Refactors the BuildConfig type to use a discriminated union for better type safety
  • Initializes outputFileName in the constructor using the specification's method or defaults to empty string

How to test your changes?

  1. Create extensions of different types (UI extensions, functions, etc.)
  2. Verify that the output file names are generated correctly for each extension type
  3. Test the build process to ensure extensions are built with the expected output file names
  4. Confirm that existing functionality remains unchanged

Measuring impact

How do we know this change was effective? Please choose one:

  • n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes

This was referenced Mar 10, 2026
Merged
Closed
Closed
Merged
Merged
@alfonso-noriega Graphite App
Copy link
Copy Markdown
Contributor Author

alfonso-noriega commented Mar 10, 2026 •
edited
Loading

This stack of pull requests is managed by Graphite. Learn more about stacking.

@alfonso-noriega alfonso-noriega mentioned this pull request Mar 10, 2026
Closed
5 tasks
@alfonso-noriega alfonso-noriega marked this pull request as ready for review March 10, 2026 10:30
@alfonso-noriega alfonso-noriega requested a review from a team as a code owner March 10, 2026 10:30
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 10, 2026

We detected some changes at packages/*/src and there are no updates in the .changeset.
If the changes are user-facing, run pnpm changeset add to track your changes and include them in the next release CHANGELOG.

Caution

DO NOT create changesets for features which you do not wish to be included in the public changelog of the next CLI release.

@binks-code-reviewer
Copy link
Copy Markdown

binks-code-reviewer bot commented Mar 10, 2026 •
edited
Loading

🤖 Code Review · #projects-dev-ai for questions
React with 👍/👎 or reply — all feedback helps improve the agent.

✅ Complete - No issues

📋 History

✅ No issues → ✅ 2 findings → ❌ Failed → ✅ No issues

@alfonso-noriega alfonso-noriega changed the base branch from 03-10-create_abstract_client_steps_infrastracture_to_externalize_the_ap_modules_client_configurations to graphite-base/6967 March 10, 2026 11:05
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch from bbc0817 to 7b68eb7 Compare March 10, 2026 11:05
@alfonso-noriega alfonso-noriega changed the base branch from graphite-base/6967 to main March 10, 2026 11:05
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 10, 2026 •
edited
Loading

Coverage report

St.❔
 Category Percentage Covered / Total
🟢 Statements 82.03% 14718/17943
🟡 Branches 74.4% 7293/9803
🟢 Functions 81.23% 3735/4598
🟢 Lines 82.43% 13919/16886

Test suite run success

3833 tests passing in 1480 suites.

Report generated by 🧪jest coverage report action from 2151198

@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch from 7b68eb7 to c3ee349 Compare March 10, 2026 11:24
binks-code-reviewer[bot]
binks-code-reviewer bot reviewed Mar 10, 2026
View reviewed changes
Comment thread packages/app/src/cli/models/extensions/specifications/checkout_ui_extension.ts Outdated
binks-code-reviewer[bot]
binks-code-reviewer bot reviewed Mar 10, 2026
View reviewed changes
Comment thread packages/app/src/cli/models/extensions/specifications/function.ts Outdated
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch 2 times, most recently from 24fb214 to ecf7d0f Compare March 10, 2026 12:41
ryancbahan
ryancbahan reviewed Mar 10, 2026
View reviewed changes
Comment thread packages/app/src/cli/models/extensions/extension-instance.ts Outdated
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch 2 times, most recently from 9bebf7c to dd63de8 Compare March 12, 2026 10:30
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch 4 times, most recently from 7e38d07 to 880b269 Compare March 12, 2026 13:19
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch 2 times, most recently from 5647756 to dcffdf5 Compare March 16, 2026 11:17
This was referenced Mar 16, 2026
Merged
Closed
isaacroldan
isaacroldan reviewed Mar 16, 2026
View reviewed changes
Comment thread packages/app/src/cli/models/extensions/extension-instance.ts Outdated
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch from dcffdf5 to b7dd050 Compare March 17, 2026 10:08
@alfonso-noriega alfonso-noriega mentioned this pull request Mar 17, 2026
Merged
5 tasks
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch 2 times, most recently from a8cf663 to 40c8534 Compare March 17, 2026 11:07
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch from 40c8534 to 72e5cc8 Compare March 18, 2026 10:42
@alfonso-noriega alfonso-noriega force-pushed the 03-10-clean_up_module_outputfile_calculation_and_move_it_to_the_specifications branch from 72e5cc8 to 2151198 Compare March 18, 2026 17:50
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 19, 2026

This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action.
→ If there's no activity within a week, then a bot will automatically close this.
Thanks for helping to improve Shopify's dev tooling and experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@binks-code-reviewer binks-code-reviewer[bot] binks-code-reviewer[bot] left review comments

@ryancbahan ryancbahan Awaiting requested review from ryancbahan

@isaacroldan isaacroldan Awaiting requested review from isaacroldan

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

no-pr-activity

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@alfonso-noriega @isaacroldan @ryancbahan